-
Notifications
You must be signed in to change notification settings - Fork 748
Revert "Use std::optional" #15612
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Revert "Use std::optional" #15612
Conversation
This reverts commit 3f4f500.
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/15612
Note: Links to docs will display an error until the docs builds have been completed. ⏳ No Failures, 12 PendingAs of commit ece021b with merge base 149e23d ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
This PR needs a
|
|
cc @cyyever I think the root cause of the failures is what I mentioned on your follow up diff of we cant just remove the name "torch::executor::optional" you will need to leave that exposed as an alias for std::optional |
I think you just need to add this line: To runtime/core/exec_aten/exec_aten.h |
Reverts pytorch#12707 This causes 1000s of internal CI failures, e.g., error: no template named 'optional' in namespace 'torch::executor'; did you mean simply 'optional'? using optional = torch::executor::optional<T>; ^~~~~~~~~~~~~~~~~~~~~~~~~ optional note: 'optional' declared here using ::executorch::aten::optional;
|
@JacobSzwejbka Is it better to migrate internal tests to std::optional before changing OSS? |
Reverts #12707
This causes 1000s of internal CI failures, e.g.,
error: no template named 'optional' in namespace 'torch::executor'; did you mean simply 'optional'?
using optional = torch::executor::optional;
^~~~~~~~~~~~~~~~~~~~~~~~~
optional
note: 'optional' declared here
using ::executorch::aten::optional;